PCRE2: optimize memory allocations #15395

ysbaddaden · 2025-01-31T15:03:13Z

We noticed in #15088 that we don't need Crystal::ThreadLocalValue in the Regex PCRE2 engine.

We can reuse the JIT stack but also the match data for every Regex (no need for a specific match data per Regex). We can allocate one and make sure it's not used by other threads... hence the thread locals: no more spinlock (thread contention) nor hash.

It's simpler and faster. Here's the benchmark from #13144 for example:

$ crystal run --release bench/regex.cr
starts_with?  29.46M ( 33.94ns) (± 0.27%)
    matches?  40.34M ( 24.79ns) (± 0.74%)
$ bin/crystal run --release bench/regex.cr
starts_with?  30.70M ( 32.58ns) (± 0.26%)
    matches?  46.28M ( 21.61ns) (± 0.72%)

Enabling MT also no longer has any impact on performance:

$ crystal run --release -Dpreview_mt bench/regex.cr
starts_with?  26.50M ( 37.73ns) (± 0.09%)
    matches?  41.75M ( 23.95ns) (± 0.56%)
$ bin/crystal run --release -Dpreview_mt bench/regex.cr
starts_with?  30.61M ( 32.67ns) (± 0.36%)
    matches?  48.45M ( 20.64ns) (± 1.51%)

The drawback is that we must allocate each matchdata with a maximum number of ovectors (65535). That might increase memory usage, though I failed to notice it in practice. Maybe not allocating memory for every regular expression is helping?

Note: this PR will be separated into a couple PRs to introduce Thread::Local(T) separately. The point of this new type is for this patch, so I want approval on the overall approach before the split.

I could have used @[ThreadLocal] but some targets don't support it (namely: Android, MinGW and OpenBSD) and we can't register destructors either (on thread shutdown). But using pthread_key_create or FlsAlloc we can 😍

Note 2: using a type and a system destructor allows to avoid adding a property to Thread that would then need a finalizer that would hardlink and force require PCRE2 into all Crystal applications again. Using system destructors also means that the destructor will always run, even if the thread is cancelled or whatever.

Note 3: I'm wondering if this Thread::Local(T) type ain't deprecating the @[ThreadLocal] annotation: it works everywhere and the annotation doesn't even spare a function call —the codegen marks it as noinline to avoid cache issues.

ysbaddaden · 2025-02-04T13:12:09Z

Damn, that was promising, and it's working well on Linux, but both Darwin and Windows completely broke 😭

Windows could be explained by using FlsAlloc instead of TlsAlloc, but Darwin?

ysbaddaden · 2025-02-04T13:30:49Z

WTH: I can't reproduce the errors in my Windows VM (MSVC) 😕

ysbaddaden · 2025-02-04T16:58:27Z

I can reproduce when running the compiler specs. Looking at the memory graph, the committed memory keeps growing rapidly, and it starts failing exactly when reaching 32GB (the VM has 16GB of RAM). That can't be a coincidence.

When running the compiler specs from master, the committed memory is rather stable or grows slowly (around 3.5GB). Let's note that a linker command still fails, but I get a bunch of weird spec errors in the VM.

Maybe FlsAlloc ain't exactly compatible with our fiber-context switch (argh)?

Some targets don't support the ThreadLocal annotation (for example OpenBSD, Android)

…ermination

ysbaddaden · 2025-03-11T07:56:42Z

Here's a new attempt.

From CI runs in my fork, the darwin and win32-msvc targets got fixed, but win32-gnu is still broken on dual heredocs such as call <<-CRYSTAL, <<-CRYSTAL which I don't understand 😞

Note: I believe FLS should work on Windows, which would avoid the custom destructors. The stdlib specs pass, save for these failures in the compiler specs.

HertzDevil · 2025-03-11T08:15:05Z

Yeah that's the LLVM 20 failure, ignore it for the moment

ysbaddaden · 2025-03-11T12:13:11Z

I tried again, and FLS is apparently working on Windows. Nice.

ysbaddaden · 2025-03-11T12:14:33Z

src/crystal/system/thread.cr

      Thread.threads.delete(self)
      Fiber.inactive(fiber)
      detach { system_close }
    end
  end

+  protected def run_destructors : Nil
+  end


todo: cleanup before merge (not needed anymore).

src/regex/pcre2.cr

src/crystal/system/win32/thread_local.cr

src/crystal/system/thread_local.cr

Sija · 2025-03-15T18:20:05Z

src/crystal/system/thread_local.cr

+    # def initialize
+    #   {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+    # end
+
+    # def initialize(&destructor : Proc(T, Nil))
+    #   {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+    # end


Perhaps these could be uncommented and then called via super in descendant classes to avoid duplicating the code?

Suggested change

# def initialize

# {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

# end

# def initialize(&destructor : Proc(T, Nil))

# {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

# end

def initialize

{% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

end

def initialize(&destructor : Proc(T, Nil))

initialize

end

It avoids some repetition, but I'm not sure it would improve readability?

I'm wondering if we could use Box for this? Seems to be exactly the use case. Technically we don't need support for boxing structs onto the heap though. So this draws in additional complexity which we might want to avoid here.

Sorry, you lost me @straight-shoota 😕

~~Do you mean to Box the destructor proc?~~ Oh, to Box the value in case it's not a Pointer/Reference! 😅

I think it's fine to not support non pointer values. If we ever need to, we'll figure it out then.

The idea for using Box would be that it might simplify the code for Thread::Local. All explicit casts would become Box(T).box and Box(T).unbox and no need for checking T type in the macro.
I suppose it doesn't simplify that much though.

But Box always allocates into the GC heap, and we'd put the pointer to the box in the local storage that we'd have to dereference on every get 😕

Errata: Box doesn't allocate into the GC heap when T is Nil or Reference but we want to store pointers and that will allocate.

Maybe it's an overlook? 🤔

straight-shoota · 2025-03-17T10:55:26Z

src/crystal/system/unix/thread_local.cr

+    def initialize
+      {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+      err = LibC.pthread_key_create(out @key, nil)
+      raise RuntimeError.from_os_error("pthread_key_create", Errno.new(err)) unless err == 0
+    end
+
+    def initialize(&destructor : Proc(T, Nil))
+      {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+      err = LibC.pthread_key_create(out @key, destructor.unsafe_as(Proc(Void*, Nil)))
+      raise RuntimeError.from_os_error("pthread_key_create", Errno.new(err)) unless err == 0
+    end


suggestion: Extract the common code into a helper to remove duplication.

Suggested change

def initialize

{% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

err = LibC.pthread_key_create(out @key, nil)

raise RuntimeError.from_os_error("pthread_key_create", Errno.new(err)) unless err == 0

end

def initialize(&destructor : Proc(T, Nil))

{% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

err = LibC.pthread_key_create(out @key, destructor.unsafe_as(Proc(Void*, Nil)))

raise RuntimeError.from_os_error("pthread_key_create", Errno.new(err)) unless err == 0

end

def initialize

create_key

end

def initialize(&destructor : Proc(T, Nil))

create_key destructor

end

private def create_key(destructor = nil)

{% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

err = LibC.pthread_key_create(out @key, destructor.unsafe_as(Proc(Void*, Nil)))

raise RuntimeError.from_os_error("pthread_key_create", Errno.new(err)) unless err == 0

end

ditto for win32

straight-shoota · 2025-03-17T10:56:59Z

src/crystal/system/win32/thread_local.cr

+    def initialize
+      {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+      @key = LibC.FlsAlloc
+      raise RuntimeError.from_winerror("FlsAlloc: out of indexes") if @key == LibC::FLS_OUT_OF_INDEXES
+    end
+
+    def initialize(&destructor : Proc(T, Nil))
+      {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+      @key = LibC.FlsAlloc(destructor.unsafe_as(LibC::FLS_CALLBACK_FUNCTION))
+      raise RuntimeError.from_winerror("FlsAlloc: out of indexes") if @key == LibC::FLS_OUT_OF_INDEXES
+    end


Suggested change

def initialize

{% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

@key = LibC.FlsAlloc

raise RuntimeError.from_winerror("FlsAlloc: out of indexes") if @key == LibC::FLS_OUT_OF_INDEXES

end

def initialize(&destructor : Proc(T, Nil))

{% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

@key = LibC.FlsAlloc(destructor.unsafe_as(LibC::FLS_CALLBACK_FUNCTION))

raise RuntimeError.from_winerror("FlsAlloc: out of indexes") if @key == LibC::FLS_OUT_OF_INDEXES

end

def initialize

create_key nil

end

def initialize(&destructor : Proc(T, Nil))

create_key destructor

end

private def create_key(destructor)

{% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}

@key = LibC.FlsAlloc(destructor.unsafe_as(LibC::FLS_CALLBACK_FUNCTION))

raise RuntimeError.from_winerror("FlsAlloc: out of indexes") if @key == LibC::FLS_OUT_OF_INDEXES

end

straight-shoota · 2025-03-17T10:59:35Z

src/crystal/system/thread_local.cr

+    # def initialize
+    #   {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+    # end
+
+    # def initialize(&destructor : Proc(T, Nil))
+    #   {% raise "T must be a Reference or Pointer" unless T < Reference || T < Pointer %}
+    # end


The idea for using Box would be that it might simplify the code for Thread::Local. All explicit casts would become Box(T).box and Box(T).unbox and no need for checking T type in the macro.
I suppose it doesn't simplify that much though.

ysbaddaden · 2025-03-17T17:35:57Z

Moving back to draft. I'll be pushing individual PRs with the changes proposed here:

Fix: Box(T).box allocates when T < Pointer #15562
Add Thread::Local(T) using Box(T)
PCRE2: single jit stack/match data allocation per thread
bonus: simplify Thread.current

A branch with all the changes is available at https://github.com/ysbaddaden/crystal/tree/feature/add-thread-local-struct

ysbaddaden self-assigned this Jan 31, 2025

ysbaddaden changed the title ~~Refactor: PCRE2 memory allocations~~ PCRE2: optimize memory allocations Jan 31, 2025

Blacksmoke16 mentioned this pull request Jan 31, 2025

Adjust PCRE2 monkey patches athena-framework/athena#510

Open

ysbaddaden added 12 commits March 10, 2025 14:55

Regex::PCRE2: set jit stack and match data as ThreadLocal

f24a948

LibC: add bindings for pthread local storage

93e141f

WIP: Thread::Local(T) struct

4aeeb47

Regex::PCRE2: use Thread::Local(T) instead of ThreadLocal annotation

b6ebabc

Some targets don't support the ThreadLocal annotation (for example OpenBSD, Android)

WIP: extract Thread::Local(T) implementations

621fe5f

LibC: add bindings for windows FLS API

a14032e

fixup! WIP: extract Thread::Local(T) implementations

f5da032

WIP: try FlsAlloc on windows

c225013

Add destructor to Thread::Local(T)

4b0accf

Fix: register destructor to free jit stack and match data on thread t…

64458fb

…ermination

fixup! Add destructor to Thread::Local(T)

19f631b

Windows: go back to TlsAlloc with custom destructor array

7487cb5

ysbaddaden force-pushed the refactor/pcre2-memory-allocations branch from 7ceca47 to 7487cb5 Compare March 11, 2025 07:43

ysbaddaden added 2 commits March 11, 2025 08:50

Fix: initializer on wasi

0cbd5e8

Fix: crystal tool format

9fe3a62

Try FLS on windows again?

72961ca

ysbaddaden commented Mar 11, 2025

View reviewed changes

ysbaddaden marked this pull request as ready for review March 11, 2025 13:36

straight-shoota reviewed Mar 13, 2025

View reviewed changes

src/regex/pcre2.cr Outdated Show resolved Hide resolved

src/regex/pcre2.cr Show resolved Hide resolved

src/crystal/system/win32/thread_local.cr Outdated Show resolved Hide resolved

src/crystal/system/thread_local.cr Outdated Show resolved Hide resolved

straight-shoota added kind:refactor topic:stdlib:text labels Mar 13, 2025

straight-shoota added the topic:multithreading label Mar 13, 2025

mverzilli approved these changes Mar 14, 2025

View reviewed changes

Sija reviewed Mar 15, 2025

View reviewed changes

ysbaddaden added 2 commits March 17, 2025 10:36

Apply review suggestions

52f908d

cleanup

8688b4b

straight-shoota reviewed Mar 17, 2025

View reviewed changes

ysbaddaden mentioned this pull request Mar 17, 2025

Fix: Box(T).box allocates when T < Pointer #15562

Open

ysbaddaden marked this pull request as draft March 17, 2025 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PCRE2: optimize memory allocations #15395

PCRE2: optimize memory allocations #15395

ysbaddaden commented Jan 31, 2025 •

edited

Loading

ysbaddaden commented Feb 4, 2025

ysbaddaden commented Feb 4, 2025 •

edited

Loading

ysbaddaden commented Feb 4, 2025

ysbaddaden commented Mar 11, 2025 •

edited

Loading

HertzDevil commented Mar 11, 2025

ysbaddaden commented Mar 11, 2025

ysbaddaden Mar 11, 2025

Sija Mar 15, 2025 •

edited

Loading

ysbaddaden Mar 17, 2025

straight-shoota Mar 17, 2025

ysbaddaden Mar 17, 2025 •

edited

Loading

ysbaddaden Mar 17, 2025

straight-shoota Mar 17, 2025

ysbaddaden Mar 17, 2025

ysbaddaden Mar 17, 2025

straight-shoota Mar 17, 2025 •

edited

Loading

straight-shoota Mar 17, 2025

straight-shoota Mar 17, 2025

ysbaddaden commented Mar 17, 2025

PCRE2: optimize memory allocations #15395

Are you sure you want to change the base?

PCRE2: optimize memory allocations #15395

Conversation

ysbaddaden commented Jan 31, 2025 • edited Loading

ysbaddaden commented Feb 4, 2025

ysbaddaden commented Feb 4, 2025 • edited Loading

ysbaddaden commented Feb 4, 2025

ysbaddaden commented Mar 11, 2025 • edited Loading

HertzDevil commented Mar 11, 2025

ysbaddaden commented Mar 11, 2025

Choose a reason for hiding this comment

Sija Mar 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ysbaddaden Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

straight-shoota Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ysbaddaden commented Mar 17, 2025

ysbaddaden commented Jan 31, 2025 •

edited

Loading

ysbaddaden commented Feb 4, 2025 •

edited

Loading

ysbaddaden commented Mar 11, 2025 •

edited

Loading

Sija Mar 15, 2025 •

edited

Loading

ysbaddaden Mar 17, 2025 •

edited

Loading

straight-shoota Mar 17, 2025 •

edited

Loading